Clean up code for drawing overlays to accept sRGB hex codes instead of Color structs#3839
Clean up code for drawing overlays to accept sRGB hex codes instead of Color structs#3839
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the color handling mechanism for drawing overlays throughout the editor. The primary goal is to streamline the process by transitioning from using Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/messages/portfolio/document/overlays/utility_types_native.rs">
<violation number="1" location="editor/src/messages/portfolio/document/overlays/utility_types_native.rs:1067">
P2: Duplicated hex parsing logic — extract a shared `parse_color_bytes(color: &str) -> [u8; 4]` helper and have both `parse_color` and `fill_path_pattern` call it. This avoids maintaining the same parsing in two places.</violation>
</file>
<file name="editor/src/messages/portfolio/document/utility_types/misc.rs">
<violation number="1" location="editor/src/messages/portfolio/document/utility_types/misc.rs:218">
P1: Missing `#[serde(alias = "grid_color")]` on the renamed `color` field. Existing serialized documents (including all 8 demo `.graphite` files) store this value as `grid_color`. Without the alias, deserialization will silently discard the old value and replace it with the default, losing the user's saved grid color setting. This file already uses `#[serde(alias = ...)]` for the same purpose on `GridType::Rectangular`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a nice cleanup that simplifies color handling for overlays by using sRGB hex codes instead of Color structs. The changes generally make the code cleaner and more efficient.
However, I've identified a critical issue where the new hex string parsing logic can cause a panic with malformed input, potentially crashing the application. This unsafe logic is duplicated in two files. Additionally, there's a color correctness bug in the gradient tool where colors are not being properly gamma-corrected before display. I've provided detailed comments and suggestions for these issues.
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/messages/portfolio/document/overlays/utility_functions.rs">
<violation number="1" location="editor/src/messages/portfolio/document/overlays/utility_functions.rs:255">
P2: Byte-index string slicing (`&hex[0..2]`) can panic on non-ASCII input that happens to be exactly 6 or 8 UTF-8 bytes (e.g., `"你好"` is 6 bytes). Add an ASCII check before slicing to maintain the graceful fallback behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
1 participant
No description provided.